-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve KBS protocol version handling and bump the version to v0.1.1 due to kbs-types changes #628
Conversation
622fe89
to
39ed94d
Compare
whoops, didn't mean to make this ready for review |
This PR now depends on getting confidential-containers/trustee#449 merged and released with |
|
10a95c5
to
783b154
Compare
@Xynnn007 PTAL also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @mythi.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in reviewing. Seems good. One suggestion
Signed-off-by: Mikko Ylinen <[email protected]>
The RCAR client code currently ignores any errors for "request". Such errors can still happen, e.g., when 'version' field is rejected by KBS. Without catching errors we try to decode the Challenge json body but it actually contains the error information in it which results in decode errors instead. KBS added a new ProtocolVersion error which is now catched by the RCAR client code. The error is reported to the user if the client and server use incompatible versions. Signed-off-by: Mikko Ylinen <[email protected]>
The current setup with trustee repo for KBS server and guest-components repo for kbs_protocol client code has a cyclic dependency problem. test_client test uses "KBS latest" container image which won't work if any RCAR client code changes that change the protocol semantics are made. The suggested approach to break the cyclic dependency is to get the RCAR client changes merged first with a KBS protocol version bump. The RCAR client test is modified to skip the checks in case the "KBS latest" server returns ProtocolVersion error. Following this, the corresponding KBS server changes are made in trustee repo with an update to both kbs_protocol for kbs-client builds and the server supported protocol version. Signed-off-by: Mikko Ylinen <[email protected]>
kbs-types introduces a break in JSON semantics so we bump the kbs protocol version from 0.1.0 to 0.1.1. Signed-off-by: Mikko Ylinen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pull in @Xynnn007's work for confidential-containers/trustee#242 (comment)